Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration branch ux 12.2024 #9765

Merged
merged 7 commits into from
Dec 4, 2024
Merged

Conversation

KManolov3
Copy link
Contributor

Resolves

What Github issue does this resolve (please include link)?

  • Resolves #

Proposed Changes

Describe what this Pull Request does

Reason for Changes

Explain why these changes should be made

Test Coverage

Please show how you have added tests to cover your changes

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

  • Chrome
  • Firefox
  • Safari

Windows

  • Chrome
  • Firefox
  • Edge

Chromebook

  • Chrome

iPad

  • Safari

Android Tablet

  • Chrome

Copy link

github-actions bot commented Dec 4, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@KManolov3
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@KManolov3
Copy link
Contributor Author

recheck

@KManolov3 KManolov3 merged commit 68f5adc into develop Dec 4, 2024
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2024
Comment on lines +35 to +39
useEffect(() => {
sections.forEach(section => {
new Image().src = section.image;
});
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this pattern... could you explain what's going on here, just for my education?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is, that by loading the image once, React caches it and can reuse it later. So the earlier we execute this, the more time we have to load all images we need - in this case, on first render.
I added this at a relatively late point in developing the debug modal since I noticed an issue with the image load time. There seemed to be a noticeable improvement after that (though I did not measure it). Interestingly, I learned about this pattern from Scratch - I noticed it is used in a few places in scratch-www

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are some gaps between parts of the C blocks on this one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be an arrowhead on the orange squiggle? If this is as intended then cool :)

id: 'gui.debugModal.addSoundCheckpoints.description1',
defaultMessage:
// eslint-disable-next-line max-len
'Similar to the Slow It Down strategy, you can add different sounds with the “play until done” block at key points to test your sequence.',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space in add different

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants